Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

crypto and key-management api for DeepKey: Keystore & PassphraseManager #1104

Merged
merged 87 commits into from
Mar 22, 2019

Conversation

zippy
Copy link
Member

@zippy zippy commented Mar 9, 2019

This PR

  1. creates a Keystore implementation in dpki crate which has the functions necessary for crypto to be usable both by DPKI and other apps that want to do secure crypto functions. The work is based on this spec:
    https://hackmd.io/tVy22HhJQPuHO28gtD_UTQ?both
    and this diagram:
    https://realtimeboard.com/app/board/o9J_kycnaks=/
  2. creats a PassphraseManager service/trait for getting passphrases from somewhere (either testing environments, or UIs)
  3. Makes Keystores persistable (with encryption using the passphrase)

What it does not do, and needs to be done in future prs is:

  1. Actually implement a UI passphrase service
  2. Wrap the keystore functions in hdk functions available to running in zomes.
  3. Follow the designed bootup sequence, using an instance of DeepKey to actually generate agent keys instead of using our current hc keygen solution
  • I have added a summary of my changes to the changelog

@zippy zippy added the review label Mar 9, 2019
@zippy zippy requested review from lucksus, zo-el and neonphog and removed request for lucksus March 12, 2019 21:25
@zippy zippy changed the title WIP: crypto and key-management api for DeepKey crypto and key-management api for DeepKey Mar 12, 2019
dpki/src/seed.rs Outdated Show resolved Hide resolved
dpki/src/key_blob.rs Outdated Show resolved Hide resolved
dpki/src/key_bundle.rs Outdated Show resolved Hide resolved
dpki/src/keypair.rs Outdated Show resolved Hide resolved
dpki/src/seed.rs Outdated Show resolved Hide resolved
dpki/src/keystore.rs Outdated Show resolved Hide resolved
dpki/src/keystore.rs Outdated Show resolved Hide resolved
@lucksus lucksus changed the title WIP: crypto and key-management api for DeepKey: Keystore & PassphraseManager crypto and key-management api for DeepKey: Keystore & PassphraseManager Mar 20, 2019
}
};
self.cache
.insert(id_str.clone(), Arc::new(Mutex::new(secret)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm uncertain about having a cache here. Part of the point of the passphrase manager is to give users control over "locking" their authorizor capabilities after an expiration timer, but if they authorize something and their authorizor key gets cached here, then we have to coordinate expiring the cache when the passphrase expires?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I get that.

But we do need to hold some secrets unencrypted indefinitely: agent keys in particular.

I would say we might need to add more complexity around this cache so that agent keys stay in, even after the PassphraseManager locks itself up. But everything else gets cleared from the cache at some point, as a default that applies in particular to seeds.. Maybe dependent on the secret type? Or everything gets cleared except for PRIMARY_KEYBUNDLE_ID? What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I was imagining anything that needed persistence like for the app keys handling it on that end... i.e. keeping a copy of the secret after requesting it. But definitely something we can iterate on to see what works.

Copy link
Contributor

@neonphog neonphog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍 This is starting to come together! Couple questions / change requests above.

@zippy zippy requested a review from neonphog March 21, 2019 18:28
Copy link
Contributor

@neonphog neonphog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wooo, crypto incoming! lgtm

@zippy zippy merged commit 385b358 into develop Mar 22, 2019
Copy link
Member

@zo-el zo-el left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@zippy zippy deleted the crypto-conductor-1 branch July 5, 2019 13:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants